Is specifying all zones required for StandardV2? If so please add a CustomizeDiff validator for this.

Refer to:

StandardV2 NAT Gateway is zone-redundant, meaning that it provides outbound connectivity from all zones in a region instead of a single zone like Standard NAT Gateway.

https://learn.microsoft.com/en-us/azure/nat-gateway/nat-overview

Reason1: I think we should not use a CustomizeDiff validator to force users to specify zones, because the API also allows it when users do not specify them.

Reason2: About ignoring zones, according to the previous recommendations provided by HC, when the configuration of property A affects other properties—for example, property B—the affected property B should be handled by the user in the Terraform configuration. This allows users to clearly understand that when property A is modified, property B will also be impacted, rather than suppressing the issue through changes in the provider, which could cause property B to change without the user being aware of it. This can prevent users from encountering unexpected behavior. So, I add the comment for it.

Please implement a CustomizeDiff validation to force user to set zones to [1, 2 ,3] if StandardV2 SKU is used. A documentation note like this is not enough as user can miss it.

Also please reword this sentence to: 'StandardV2' SKU require zones to be set to [1,2,3], see [MS learn documentation](link to relevant doc) for more info

I added CustomizeDiff validation and updated the md file. Thanks.

Can be simplified using slices.Equal(zones, []string{"1", "2", "3"})

Updated

We should update this to use the standard wording per the contributor guidelines.

updated

Is this correct? I thought the documentation said this had to be Standard.

By tests, StandardV2 can also set with Global sku_tier. Actually it should be
-> Note: When sku_tier is set to Global, sku cannot be Basic

Hi @teowa this is a good catch. We don't actually support Global tier on V2 IPs. We will add a validation from our side to block this. Only Standard SKU IPs support Global tier.

I don't think we should start limiting input to zone/zones arguments. There is at least one region that supports 4 (eastus2euap, while a bit of a special case, users could be using it)

Updated; now we just check that it isn't an empty string.

from PG here - new creates of Basic SKU public IP resources as of March 2025 deprecation date is not supported. We are working on blocking in NRP, just hasn't rolled out yet.

I'm a little confused as to why this function is here, isPublicIPBasicSkuDeprecatedForCreation() is already true so there's no need for it?

updated, this is removed.

I don't think this part is relevant given this date has passed

Existing Basic SKU public IP addresses can continue to be used until September 30, 2025.

updated

I believe this also applies to StandardV2 so this check needs updating

updated, allocation_method must be set to Static when sku is set to Standard or StandardV2

This date is in the past, so all the tests using this function are already irrelevant? (though they are still passing now)

Despite being past the deprecation date, it is still possible to allocate a Basic SKU public IP.

Yes, all tests that use sku=Basic for azurerm_public_ip will need to be migrated. These are being addressed in PR #31885.

Despite being past the deprecation date, it is still possible to allocate a Basic SKU public IP.

new creates of Basic SKU public IP resources as of March 2025 deprecation date is not supported. Azure service team is working on blocking in NRP, just hasn't rolled out yet.

Since the tests aren't failing yet and the function will always return true, can you remove this in favour of the migration in #31885?

updated, this is removed now

minor rephrasing that may make it more clear, WDYT?

updated

Date already passed? This would probably make more sense as something like `sku` can no longer be set to `Basic` as of <date> for new resources"

updated

Should this be validated for?

updated, When sku_tier is set to Global, sku must be set to Standard.

Making this O+C breaks the existing behaviour of replacing the resource when a user removes zones from their deployment (when sku is Standard). We should try to keep this behaviour intact and we could use a DiffSuppressFunc instead.

Additionally, O+C doesn't entirely resolve the zones diff, currently a user could specify zones for StandardV2 in config (e.g. zones = ["1"]) which would lead to a perpetual diff, we should prevent users from setting zones on StandardV2 with a CustomizeDiff

e.g.

DiffSuppress:

		DiffSuppressOnRefresh: true,
			DiffSuppressFunc: func(_, _, _ string, d *schema.ResourceData) bool {
				// when `sku_name` is `StandardV2`, Azure automatically spreads the deployment across all zones in the region
				return d.Get("sku_name").(string) == string(natgateways.NatGatewaySkuNameStandardVTwo)
			},

CustomizeDiff:

		CustomizeDiff: func(_ context.Context, diff *schema.ResourceDiff, _ interface{}) error {
			if diff.Get("sku_name").(string) == string(natgateways.NatGatewaySkuNameStandardVTwo) {
				if !diff.GetRawConfig().AsValueMap()["zones"].IsNull() {
					return fmt.Errorf("%s resources with `sku_name` set to `%s` are zone-redundant by default, Azure automatically deploys across all available zones. The `zones` argument must be omitted", natGatewayResourceName, natgateways.NatGatewaySkuNameStandardVTwo)
				}
			}

			return nil
		},

Make sense, code updated.

Since the tests aren't failing yet and the function will always return true, can you remove this in favour of the migration in #31885?